Skip to content

Fix read configuration#397

Open
chiarorosa wants to merge 3 commits into
overleaf:masterfrom
chiarorosa:fix-read-configuration
Open

Fix read configuration#397
chiarorosa wants to merge 3 commits into
overleaf:masterfrom
chiarorosa:fix-read-configuration

Conversation

@chiarorosa
Copy link
Copy Markdown

@chiarorosa chiarorosa commented Oct 21, 2025

Description

This PR simplifies read_configuration() in shared-functions.sh by replacing a complex sed regex with a clearer two-step parsing:
• Old: sed -r "s/^$name=(["']?)(.+)\1$/\2/"
• New: cut -d '=' -f2- | sed -r "s/^(["']?)(.+)\1$/\2/"

Why:

The new approach is easier to read and reason about while preserving behavior:
• Extracts the value after the first =
• Strips optional matching quotes around the value

Impact & Testing:

•	Functionality unchanged; continues to read values from overleaf.rc and remove surrounding quotes
•	Verified locally: function works as expected across typical configs
•	No breaking changes observed

Related issues / Pull Requests

•	Related to bin/up command (uses configuration parsing)

Contributor Agreement

- Dockerfile: Custom Overleaf image with Portuguese LaTeX support
- .github/copilot-instructions.md: AI assistant instructions for the toolkit
- config/overleaf.rc: Custom configuration for UFPEL edition
Replace the complex sed regex with a simpler approach using cut -d '=' -f2-
to extract the value after the equals sign, followed by sed to remove quotes.

This makes the function more readable and potentially more robust.
These files belong to the custom-ufpel-edition branch and should not
be included in the PR for the read_configuration fix.
@mserranom
Copy link
Copy Markdown
Contributor

Looks like a good solution for #282. WDYT @das7pad?

@das7pad
Copy link
Copy Markdown
Member

das7pad commented May 19, 2026

So the effective change is removing the escaping from \$?

@mserranom
Copy link
Copy Markdown
Contributor

So the effective change is removing the escaping from $?

No, it's removing ^$name= from the regexp, and stripping the name with cut beforehand.

@das7pad
Copy link
Copy Markdown
Member

das7pad commented May 19, 2026

So the effective change is removing the escaping from $?

No, it's removing ^$name= from the regexp, and stripping the name with cut beforehand.

Which is effectively the same operation 😉

@mserranom
Copy link
Copy Markdown
Contributor

Yeah, fair point. Would you mind stamping ✅ if you're also fine with it?

Copy link
Copy Markdown
Member

@das7pad das7pad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, not tested.

There are unrelated changes in this PR. Do you mind force-pushing them away @chiarorosa ?

Also please fill out the CLA and tick the box on the PR. I've restored the section in the PR description.

@das7pad das7pad assigned chiarorosa and unassigned das7pad May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants